Skip to content

Remove McpServerConfig and have McpClientFactory accept IClientTransport instances directly.#230

Merged
eiriktsarpalis merged 5 commits intomodelcontextprotocol:mainfrom
eiriktsarpalis:rework-client
Apr 8, 2025
Merged

Remove McpServerConfig and have McpClientFactory accept IClientTransport instances directly.#230
eiriktsarpalis merged 5 commits intomodelcontextprotocol:mainfrom
eiriktsarpalis:rework-client

Conversation

@eiriktsarpalis
Copy link
Copy Markdown
Member

Posting as draft to get early feedback on the direction being taken here.

Comment thread src/ModelContextProtocol/Protocol/Transport/SseClientTransportOptions.cs Outdated
Comment thread src/ModelContextProtocol/Protocol/Transport/StdioClientTransportOptions.cs Outdated
Comment thread samples/ChatWithTools/Program.cs Outdated
Comment thread src/ModelContextProtocol/Client/McpClient.cs Outdated
Comment thread src/ModelContextProtocol/Client/McpClientFactory.cs Outdated
Copy link
Copy Markdown
Contributor

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direction lgtm

Copy link
Copy Markdown
Contributor

@halter73 halter73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good aside from disposing the IClientTransport when ConnectAsync throws.

Comment thread src/ModelContextProtocol/Client/McpClientFactory.cs Outdated
Comment thread tests/ModelContextProtocol.Tests/SseServerIntegrationTestFixture.cs Outdated
@eiriktsarpalis eiriktsarpalis marked this pull request as ready for review April 8, 2025 14:04
Comment thread src/ModelContextProtocol/Protocol/Transport/IClientTransport.cs Outdated
Comment thread src/ModelContextProtocol/Protocol/Transport/SseClientTransportOptions.cs Outdated
Comment thread src/Common/Polyfills/System/PasteArgument.cs
Comment thread src/ModelContextProtocol/ModelContextProtocol.csproj
@eiriktsarpalis eiriktsarpalis merged commit 3e21f35 into modelcontextprotocol:main Apr 8, 2025
13 of 15 checks passed
@eiriktsarpalis eiriktsarpalis deleted the rework-client branch April 8, 2025 16:37
@jeffhandley
Copy link
Copy Markdown
Contributor

Adding the breaking-change label retroactively during release notes revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This issue or PR introduces a breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants